-
Notifications
You must be signed in to change notification settings - Fork 5.1k
test: Fix guest_env_test.go #21801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Fix guest_env_test.go #21801
Conversation
|
/ok-to-test |
|
/cc @medyagh |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The test used `go:build iso` so it was not included in the integration tests. Change to `go:build integration` so we test in the CI. Rename the file and the test name to make it more clear that this test is about the iso image. Skip the test for non vm-driver, since this tests is about the iso image. We may need to add a similar test or adapt this test so it can be used also with the kicbase image. This test will be useful to validate kubernetes#21800, avoiding regressions such as kubernetes#21788.
Virtualbox binaries are not built in the arm64 iso. Test them only on
other architectures.
Example run:
% go test -v ./test/integration -run TestISO -tags integration
--- PASS: TestISOImage (17.74s)
--- PASS: TestISOImage/Setup (17.31s)
--- PASS: TestISOImage/Binaries (0.00s)
--- PASS: TestISOImage/Binaries/docker (0.08s)
--- PASS: TestISOImage/Binaries/podman (0.09s)
--- PASS: TestISOImage/Binaries/iptables (0.10s)
--- PASS: TestISOImage/Binaries/git (0.10s)
--- PASS: TestISOImage/Binaries/curl (0.11s)
--- PASS: TestISOImage/Binaries/crictl (0.11s)
--- PASS: TestISOImage/Binaries/wget (0.06s)
--- PASS: TestISOImage/Binaries/socat (0.07s)
--- PASS: TestISOImage/Binaries/rsync (0.06s)
--- PASS: TestISOImage/PersistentMounts (0.00s)
--- PASS: TestISOImage/PersistentMounts//var/lib/boot2docker (0.08s)
--- PASS: TestISOImage/PersistentMounts//var/lib/cni (0.09s)
--- PASS: TestISOImage/PersistentMounts//var/lib/toolbox (0.09s)
--- PASS: TestISOImage/PersistentMounts//var/lib/minikube (0.10s)
--- PASS: TestISOImage/PersistentMounts//var/lib/docker (0.10s)
--- PASS: TestISOImage/PersistentMounts//data (0.11s)
--- PASS: TestISOImage/PersistentMounts//var/lib/kubelet (0.06s)
|
Test results on top of #21800 successful checks
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| // TestGuestEnvironment verifies files and packages installed inside minikube ISO/Base image | ||
| func TestGuestEnvironment(t *testing.T) { | ||
| // TestISOImage verifies files and packages installed inside minikube ISO/Base image | ||
| func TestISOImage(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding out this test never ran !!!
How about we make this a SubTest of No-Kubernetes Test ? so we spin up one No-Kuberentes VM and dont spin up any more new VMs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using --no-kubernetes makes sense. I'll check this out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should make this a sub test of TestNoKubernetes.
The test is complicated enough as is:
// Serial tests
t.Run("serial", func(t *testing.T) {
tests := []struct {
name string
validator validateFunc
}{
{"StartNoK8sWithVersion", validateStartNoK8sWithVersion},
{"StartWithK8s", validateStartWithK8S},
{"StartWithStopK8s", validateStartWithStopK8s},
{"Start", validateStartNoK8S},
{"VerifyK8sNotRunning", validateK8SNotRunning},
{"ProfileList", validateProfileListNoK8S},
{"Stop", validateStopNoK8S},
{"StartNoArgs", validateStartNoArgs},
{"VerifyK8sNotRunningSecond", validateK8SNotRunning},
}
for _, tc := range tests {
tc := tc
if ctx.Err() == context.DeadlineExceeded {
t.Fatalf("Unable to run more tests (deadline exceeded)")
}
t.Run(tc.name, func(t *testing.T) {
tc.validator(ctx, t, profile)
if t.Failed() && *postMortemLogs {
PostMortemLogs(t, profile)
}
})
}
})Adding the test as sub test will make it impossible to run the test separately like we can do with the current PR. It may not run the test if a previous unrelated test failed, and it will make it harder to maintain the tests.
What we can as should do is to start the cluster with --no-kubernetes in the iso test since we test the iso contents, and kubernetes is not required of this. This will make the test much quicker locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using --no-kubernetes now.
We don't need to start kubernetes for testing the ISO. On macOS the test
is 2.45 times faster (from 17.137 seconds to 6.974 seconds).
Before:
% go test -v ./test/integration -run TestISO -tags integration -count 1
...
--- PASS: TestISOImage (16.27s)
--- PASS: TestISOImage/Setup (15.76s)
--- PASS: TestISOImage/Binaries (0.00s)
--- PASS: TestISOImage/Binaries/crictl (0.08s)
--- PASS: TestISOImage/Binaries/podman (0.09s)
--- PASS: TestISOImage/Binaries/rsync (0.11s)
--- PASS: TestISOImage/Binaries/docker (0.11s)
--- PASS: TestISOImage/Binaries/iptables (0.11s)
--- PASS: TestISOImage/Binaries/git (0.12s)
--- PASS: TestISOImage/Binaries/socat (0.08s)
--- PASS: TestISOImage/Binaries/curl (0.08s)
--- PASS: TestISOImage/Binaries/wget (0.08s)
--- PASS: TestISOImage/PersistentMounts (0.00s)
--- PASS: TestISOImage/PersistentMounts//var/lib/toolbox (0.08s)
--- PASS: TestISOImage/PersistentMounts//var/lib/boot2docker (0.09s)
--- PASS: TestISOImage/PersistentMounts//var/lib/cni (0.09s)
--- PASS: TestISOImage/PersistentMounts//data (0.10s)
--- PASS: TestISOImage/PersistentMounts//var/lib/kubelet (0.10s)
--- PASS: TestISOImage/PersistentMounts//var/lib/minikube (0.11s)
--- PASS: TestISOImage/PersistentMounts//var/lib/docker (0.07s)
PASS
Tests completed in 16.268875292s (result code 0)
ok k8s.io/minikube/test/integration 17.137s
After:
% go test -v ./test/integration -run TestISO -tags integration -count 1
...
--- PASS: TestISOImage (5.77s)
--- PASS: TestISOImage/Setup (5.30s)
--- PASS: TestISOImage/Binaries (0.00s)
--- PASS: TestISOImage/Binaries/wget (0.09s)
--- PASS: TestISOImage/Binaries/socat (0.09s)
--- PASS: TestISOImage/Binaries/iptables (0.10s)
--- PASS: TestISOImage/Binaries/podman (0.10s)
--- PASS: TestISOImage/Binaries/crictl (0.11s)
--- PASS: TestISOImage/Binaries/git (0.11s)
--- PASS: TestISOImage/Binaries/rsync (0.07s)
--- PASS: TestISOImage/Binaries/docker (0.08s)
--- PASS: TestISOImage/Binaries/curl (0.08s)
--- PASS: TestISOImage/PersistentMounts (0.00s)
--- PASS: TestISOImage/PersistentMounts//var/lib/toolbox (0.08s)
--- PASS: TestISOImage/PersistentMounts//var/lib/boot2docker (0.09s)
--- PASS: TestISOImage/PersistentMounts//var/lib/cni (0.09s)
--- PASS: TestISOImage/PersistentMounts//var/lib/minikube (0.10s)
--- PASS: TestISOImage/PersistentMounts//data (0.10s)
--- PASS: TestISOImage/PersistentMounts//var/lib/docker (0.11s)
--- PASS: TestISOImage/PersistentMounts//var/lib/kubelet (0.07s)
PASS
Tests completed in 5.7657725s (result code 0)
ok k8s.io/minikube/test/integration 6.974s
|
@nirs: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
kvm2 driver with docker runtime Times for minikube (PR 21801) ingress: 15.9s 17.3s 20.9s 19.4s 19.8s Times for minikube (PR 21801) start: 46.0s 45.3s 44.4s 46.1s 42.6s docker driver with docker runtime Times for minikube (PR 21801) start: 24.7s 26.7s 23.7s 22.9s 22.1s Times for minikube (PR 21801) ingress: 10.7s 12.7s 10.7s 12.7s 11.7s docker driver with containerd runtime Times for minikube start: 20.9s 21.3s 20.3s 22.2s 19.8s Times for minikube (PR 21801) ingress: 20.2s 21.2s 22.2s 23.2s 21.1s |
|
Example run in Fedora Linux VM: $ go test -v ./test/integration -run TestISO -tags integration -count 1 -minikube-start-args '--driver=kvm'
...
--- PASS: TestISOImage (27.22s)
--- PASS: TestISOImage/Setup (24.54s)
--- PASS: TestISOImage/Binaries (0.00s)
--- PASS: TestISOImage/Binaries/VBoxService (0.50s)
--- PASS: TestISOImage/Binaries/wget (0.51s)
--- PASS: TestISOImage/Binaries/rsync (0.53s)
--- PASS: TestISOImage/Binaries/crictl (0.55s)
--- PASS: TestISOImage/Binaries/VBoxControl (0.56s)
--- PASS: TestISOImage/Binaries/podman (0.37s)
--- PASS: TestISOImage/Binaries/socat (0.43s)
--- PASS: TestISOImage/Binaries/iptables (0.42s)
--- PASS: TestISOImage/Binaries/git (0.44s)
--- PASS: TestISOImage/Binaries/docker (0.45s)
--- PASS: TestISOImage/Binaries/curl (0.27s)
--- PASS: TestISOImage/PersistentMounts (0.00s)
--- PASS: TestISOImage/PersistentMounts//var/lib/cni (0.45s)
--- PASS: TestISOImage/PersistentMounts//var/lib/minikube (0.46s)
--- PASS: TestISOImage/PersistentMounts//var/lib/docker (0.50s)
--- PASS: TestISOImage/PersistentMounts//data (0.51s)
--- PASS: TestISOImage/PersistentMounts//var/lib/kubelet (0.53s)
--- PASS: TestISOImage/PersistentMounts//var/lib/boot2docker (0.36s)
--- PASS: TestISOImage/PersistentMounts//var/lib/toolbox (0.35s)
PASS
Tests completed in 27.216404548s (result code 0)
ok k8s.io/minikube/test/integration 27.289s
$ go test -v ./test/integration -run TestISO -tags integration -count 1 -minikube-start-args '--driver=docker'
Found 10 cores, limiting parallelism with --test.parallel=5
=== RUN TestISOImage
iso_test.go:34: This test requires a VM driver
--- SKIP: TestISOImage (0.00s)
PASS
Tests completed in 180.348µs (result code 0)
ok k8s.io/minikube/test/integration 0.067s |
| // Run as a group so that our defer doesn't happen as tests are runnings | ||
| t.Run("Binaries", func(t *testing.T) { | ||
| for _, pkg := range []string{"git", "rsync", "curl", "wget", "socat", "iptables", "VBoxControl", "VBoxService", "crictl", "podman", "docker"} { | ||
| binaries := []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional but we can check for other things too, we put a File that has the minikube version in there
example:
$ cat /version.json
{"iso_version": "v1.37.0-1758198818-20370", "kicbase_version": "v0.0.48", "minikube_version": "v1.37.0", "commit": "a4f96d0469d67330691be52a99ff1f91e31ba77f"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! open #21815 as good first issue.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, nirs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Here are the number of top 10 failed tests in each environments with lowest flake rate.
Besides the following environments also have failed tests:
To see the flake rates of all tests by environment, click here. |
The test used
go:build isoso it was not included in the integration tests. Change togo:build integrationso we test in the CI.We may need to add a similar test or adapt this test so it can be used also with the kicbase image.
This test will be useful to validate #21800, avoiding regressions such as #21788.
Example run